Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with sticky quick toolbar positioning #2834

Merged
merged 3 commits into from Sep 29, 2017

Conversation

jasmussen
Copy link
Contributor

This PR improves the positioning of the sticky toolbar in normal blocks and the Classic Text blocks, which regressed when the general layout was refactored to have multiple scrollable areas.

Additionally, this PR fixes #1983 by pushing the Classic Text toolbar upwards when the kitchen sink is enabled:

classic text block toolbar

@jasmussen jasmussen added the [Type] Enhancement A suggestion for improvement. label Sep 29, 2017
@jasmussen jasmussen self-assigned this Sep 29, 2017
@@ -146,6 +146,10 @@
display: block;
}

.freeform-toolbar.has-advanced-toolbar {
margin-top: -85px; /* Pull upwards the classic text toolbar when enabled */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it always 85 pixels? I guess the advanced toolbar could have several rows?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried installing TinyMCE advanced, and I see two issues, when activating the plugin, I see the "menu" toolbar pushing the block's toolbar a bit. And when adding an extra row in the settings, It's not showing up as expected too.

screen shot 2017-09-29 at 09 55 11

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid observation, and good point. I don't know what we can do to fix this — I know it seems hacky, but unless we have some way to calculate the height of the fully expanded toolbar, I can't think of a way to "work perfectly" in every toolbar situation. Especially since responsiveness is going to introduce still more issues.

That said, the above seems to work fairly well considering you have three toolbars. All content is editable, all content is visible, and there is room for the toolbars. Since one key philosophy is that unselected block is preview and selected block/edit mode can show more options, although not optimal this doesn't fall outside of the principles.

Since Gutenberg comes with up to two toolbars by default, I feel like this is a decent compromise, especially since three stacked toolbars is an editor design I don't think is very good in the first place, but there for back compat purposes.

What do you think, @karmatosed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, good points, I'm ok with this tradeoff

@karmatosed
Copy link
Member

👍 to getting this in. I feel that it's better than what we have right now and that's the important thing. It also makes the toolbar seem less annoying to me, which is a good thing as reported by users.

@jasmussen jasmussen merged commit e7f1223 into master Sep 29, 2017
@jasmussen jasmussen deleted the fix/toolbar-positioning branch September 29, 2017 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opening the Kitchen Sink pushes down content
3 participants